Moving patch 1 out of bug 1294736.

That's the initial version of the files we need.

Add readme and update script. Let's keep compiler warnings off until the __always_inline issue is fixed on upstream (assuming that's going to be accepted).

Comment on attachment 8797778 [details] [diff] [review] 0001-Bug-1307570-1.1-Add-XZ-Embedded-support-configuratio.patch Review of attachment 8797778 [details] [diff] [review]: ----------------------------------------------------------------- ::: config/external/moz.build @@ +60,5 @@ > 'media/libsoundtouch', > ] > > +if CONFIG['MOZ_LINKER']: > + external_dirs += ['modules/xz-embedded'] Please don't land both patches independently, this will leave the tree in a non-buildable state on one of the changesets, making bisect more broken than usual. ::: modules/xz-embedded/moz.build @@ +4,5 @@ > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +EXPORTS += [ > + 'linux/xz.h', Why put things in a subdirectory? Especially, we'll likely end up using this on all platforms for something else, and the directory being "linux" will just be weird. ::: modules/xz-embedded/update.sh @@ +2,5 @@ > + > +# Script to update the Mozilla in-tree copy of XZ Embedded. > +# Run this within the /modules/xz-embedded directory of the source tree. > + > +MY_TEMP_DIR=`mktemp -d -t xz-embedded_update.XXXXXX` || exit 1 $() instead of `` @@ +6,5 @@ > +MY_TEMP_DIR=`mktemp -d -t xz-embedded_update.XXXXXX` || exit 1 > + > +git clone http://git.tukaani.org/xz-embedded.git ${MY_TEMP_DIR}/xz-embedded > + > +COMMIT=`(cd ${MY_TEMP_DIR}/xz-embedded && git log | head -n 1)` COMMIT=$(git -C ${MY_TEMP_DIR}/xz-embedded rev-parse HEAD) @@ +7,5 @@ > + > +git clone http://git.tukaani.org/xz-embedded.git ${MY_TEMP_DIR}/xz-embedded > + > +COMMIT=`(cd ${MY_TEMP_DIR}/xz-embedded && git log | head -n 1)` > +perl -p -i -e "s/\[commit [0-9a-f]{40}\]/[${COMMIT}]/" README.mozilla; This doesn't do anything when running the script with the current contents of README.mozilla. @@ +20,5 @@ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_crc32.c linux/ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_dec_stream.c linux/ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_dec_lzma2.c linux/ > +rm -rf ${MY_TEMP_DIR} > +hg add linux you want addremove

Merged patched and addressed comments. (In reply to Mike Hommey [:glandium] from comment #4) > Why put things in a subdirectory? Especially, we'll likely end up using this > on all platforms for something else, and the directory being "linux" will > just be weird. Putting the xz library into a subdir helps with maintaining a clean structure between our files and the external files and avoids potential conflicts (not really a danger in this case). I've renamed the subdir to lib.

(In reply to Chris AtLee [:catlee] from comment #6) > https://treeherder.mozilla.org/#/jobs?repo=try&revision=2661af69fa2c whoops, ignore the noise

Comment on attachment 8798016 [details] [diff] [review] 0001-Bug-1307570-1.2-Add-XZ-Embedded-support-configuratio.patch Review of attachment 8798016 [details] [diff] [review]: ----------------------------------------------------------------- ::: modules/xz-embedded/update.sh @@ +10,5 @@ > +COMMIT=$(git -C ${MY_TEMP_DIR}/xz-embedded rev-parse HEAD) > +perl -p -i -e "s/\[commit [0-9a-f]{40}\]/[${COMMIT}]/" README.mozilla; > + > +rm -rf lib > +mkdir lib Make that src. @@ +18,5 @@ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_lzma2.h lib/ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_stream.h lib/ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_crc32.c lib/ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_dec_stream.c lib/ > +mv ${MY_TEMP_DIR}/xz-embedded/linux/lib/xz/xz_dec_lzma2.c lib/ Catlee's try push made me think you should also add the BCJ filter decoder. They allow for better compression of code, which should be a good thing for libxul. He also added crc64, which, as it happens, is the default used by xz when compressing, so you should need it too, except if you pass -C crc32 to xz.

(In reply to Mike Hommey [:glandium] from comment #8) > Catlee's try push made me think you should also add the BCJ filter decoder. > They allow for better compression of code, which should be a good thing for > libxul. For last nightly's libxul.so: original: 73078500 xz/xz --lzma2: 19151920 xz --armthumb --lzma2: 18340900

Addressed comments. I've planned to investigate optimization options in a follow-up bug once the basic version has landed, but it makes sense to include the required files at this stage. I've ran some tests with different BCJ filters and CRC settings: Filter CRC APK size Extraction duration (avg.) [ms] none none 28375099 3105 none CRC32 " 3277 none CRC64 " 3506 ARM none 28468347 3108 ARMTHUMB none 27410547 3146 ARMTHUMB decreases the APK size by another MB, CRC32 adds ~200ms and CRC64 ~300ms to the decompression duration. Since the APK is already CRC-signed, we're not getting much by enabling CRC for the decompressed libraries. Although, with bug 1298090 decompression times are not a real concern either.

s/XZ_DEC_CRC64/XZ_USE_CRC64 There seems to be an issue running CRC64 and ARMTHUMB together. Given that there is no real benefit from using CRC64 over CRC32 and the README advises using CRC32 in embedded situations to reduce size, we shouldn't block on this issue here.

I've found the issue with enabling CRC64, I've posted a patch in bug 1294736, too, if we still want that.

Comment on attachment 8799904 [details] [diff] [review] 0001-Bug-1307570-1.3-Add-XZ-Embedded-support-configuratio.patch Review of attachment 8799904 [details] [diff] [review]: ----------------------------------------------------------------- ::: modules/xz-embedded/update.sh @@ +1,4 @@ > +#!/bin/sh > + > +# Script to update the Mozilla in-tree copy of XZ Embedded. > +# Run this within the /modules/xz-embedded directory of the source tree. You should just add `cd $(dirname $0)`, so that this requirement goes away.

Comment on attachment 8800019 [details] [diff] [review] 0002-Bug-1307570-2.0-Switch-XZ-Embedded-to-CRC64-integrit.patch Review of attachment 8800019 [details] [diff] [review]: ----------------------------------------------------------------- Please fold both patches.

Addressed comments and merged patches.

Pushed by esawin@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/5f09af81e038 [1.4] Add XZ Embedded support configuration, scripts and the initial library version. r=glandium